PCR revocation security: lazy enforcement, type-bound PCRs#34
PCR revocation security: lazy enforcement, type-bound PCRs#34adambalogh merged 33 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Implements defense-in-depth for PCR revocation in the TEERegistry Solidity contract, adding duplicate PCR prevention, enhanced PCR approval metadata (previous hash + grace period), and multi-checkpoint PCR enforcement (registration, activation, heartbeat), with corresponding test and integration workflow updates.
Changes:
- Extend
approvePCRto includepreviousPcrHash+gracePeriod, and emit an enhancedPCRApprovedevent. - Add immediate PCR revocation handling (
revokePCR) with batch deactivation support and lazy enforcement inactivateTEE/heartbeat. - Add/extend tests and integration script coverage for revocation, grace periods, and duplicate PCR prevention.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
contracts/solidity/TEERegistry.sol |
Adds PCR duplicate prevention, enhanced PCR approval metadata, PCR revocation with batch deactivation, and additional PCR checks during activation/heartbeat. |
tests/solidity/suites/tee/test/registry.js |
Adds a new test suite covering duplicate PCR prevention, PCR revocation, grace period approval/event fields, and admin-only batch deactivation. |
scripts/integration/local_tee_workflow.go |
Updates integration workflow (RPC URL, bytecode) and adds a new section + helper calls to exercise PCR revocation/grace/duplicate behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ient/og-evm into kha/pcr-revocation-security-fix
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
approvedPCRschanged frommapping(bytes32 => ApprovedPCR)tomapping(uint8 => mapping(bytes32 => ApprovedPCR)). The same PCR hash can be approved independently per TEE type.approvePCR(pcrs, version, teeType)only approves;revokePCR(pcrHash, teeType, gracePeriod)only revokes (immediate or with grace period). The oldpreviousPcrHashrotation pattern is removed.revokePCRflips a flag or sets an expiry — no gas bomb from iterating all active TEEs. Revoked PCRs are caught atactivateTEE()andheartbeat()checkpoints via_requirePCRValidForTEE._activeTEEListand_activeTEEIndexare nowmapping(uint8 => ...), so queries likegetActivatedTEEs(teeType)only return TEEs of that type.getLiveTEEs(teeType)query: returns TEEs that are activated, have a valid PCR, and a fresh heartbeat — fully verified on-chain.removeTEE(teeId): permanently deletes a TEE from all storage (indexes + mapping), for cleaning up decommissioned TEEs.onlyTEEOwnerOrAdminmodifier: extracted shared access control fordeactivateTEE,activateTEE, andremoveTEE.PCRAlreadyExistserror added to prevent silent overwrites of active PCRs.getPublicKey,getTLSCertificate,isActive,getPaymentAddressremoved — callers usegetTEE()instead.computeMessageHash: unused utility removed from contract.Design Decisions
Why lazy enforcement over automatic deactivation?
revokePCRmust always be callable, even with thousands of active TEEs_activeTEEListuntil its next heartbeat fails, but this is a stale index entry — not a security hole_requirePCRValidForTEERevocation modes:
revokePCR(hash, teeType, 0)— immediate: PCR is instantly invalid, TEEs fail on next heartbeatrevokePCR(hash, teeType, N)— grace period: TEEs keep running for N seconds, then caught lazilyPer-type active lists:
getActivatedTEEs(teeType)is O(1) storage read instead of O(n) filterFiles Changed
TEERegistry.solremoveTEE,getLiveTEEslifecycle.jsregistry.jssettlementRelay.jsTEETestHelper.sollocal_tee_workflow.goclient.gopcr.go/tee.go--tee-typeon approve,--grace-periodon revoke)TEERegistry.jsonREADME.mdTest Plan
cd tests/solidity/suites/tee && truffle test)PCRTypeMismatchreverts when registering TEE with wrong typeremoveTEEcleans up all indexes